Skip to content

fix generic invalidation in record class, for issue #3156#4027

Open
jujn wants to merge 2 commits intoalibaba:mainfrom
jujn:fix_3156_new
Open

fix generic invalidation in record class, for issue #3156#4027
jujn wants to merge 2 commits intoalibaba:mainfrom
jujn:fix_3156_new

Conversation

@jujn
Copy link
Collaborator

@jujn jujn commented Mar 14, 2026

What this PR does / why we need it?

修复了泛型字段擦除为 Number.class 时,asm 解析器未考虑真实泛型上下文而调用 readNumber(),导致小数被默认解析为 BigDecimal 从而引发 ClassCastException 的缺陷。修复逻辑是在 ObjectReaderCreatorASM 中增加校验,当字段擦除类型为 Number.class ,降级为反射模式进行动态解析。

Summary of your change

Please indicate you've done the following:

  • Made sure tests are passing and test coverage is added if needed.
  • Made sure commit message follow the rule of Conventional Commits specification.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed.

@wenshao wenshao requested a review from Copilot March 14, 2026 16:20
@wenshao
Copy link
Member

wenshao commented Mar 14, 2026

Code Review

修复思路正确,通过在 ASM 路径中检测 Number.class 擦除场景并降级到反射模式,避免了 readNumber() 将小数默认解析为 BigDecimal 导致的
ClassCastException。

建议

  1. 嵌套 if 建议合并

// 当前写法
if (fieldReader.fieldClass != fieldReader.fieldType) {
if (fieldReader.fieldClass == Number.class) {
match = false;
break;
}
}

// 建议改为(与周围代码风格一致)
if (fieldReader.fieldClass != fieldReader.fieldType
&& fieldReader.fieldClass == Number.class) {
match = false;
break;
}

  1. 测试内部类应声明为 static

Options 和 Options2 是非静态内部类,会持有外部类 Issue3156 的隐式引用,可能影响反序列化框架的实例构造。建议改为:

public static class Options { ... }
public static class Options2 { ... }

Options2 中的 Parameter 同理。

  1. 建议补充测试用例

当前只测试了 Double,建议增加 中 T 为 Integer 的场景,以及整数值的解析验证,确保不会被错误解析为 BigDecimal。

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves fastjson2’s ASM-based ObjectReader selection to avoid incorrect handling of generic Number-typed fields (e.g., T extends Number), and adds regression tests to ensure numeric values are materialized with the expected boxed type.

Changes:

  • Adds a matching guard in ObjectReaderCreatorASM to fall back from ASM readers when fieldClass is Number but the reflective fieldType carries different generic information.
  • Extends Issue3156 tests to cover generic numeric fields for both record-based (no default ctor) and POJO-based (default ctor) parameter containers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test-jdk17/src/test/java/com/alibaba/fastjson2/issues/Issue3156.java Adds new JUnit coverage for generic Number value deserialization (expects Double).
core/src/main/java/com/alibaba/fastjson2/reader/ObjectReaderCreatorASM.java Adjusts ASM eligibility checks to avoid JIT readers in a Number + generic-type mismatch scenario.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jujn
Copy link
Collaborator Author

jujn commented Mar 15, 2026

done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants